I don't feel strongly either way, I'll leave it to @buffalojoec for approval.

rebased due to conflict on svm lib modules

would this be a bad time to note that i was working on #2741 to make account_saver::collect_accounts_to_store viable to use in svm 😅

would this be a bad time to note that i was working on #2741 to make account_saver::collect_accounts_to_store viable to use in svm 😅

Sorry, I'm missing where its' used inside svm in that PR, could you provide a direct link?

sorry, i should have been clearer, #2741 doesnt use account saver in svm, it is preparation for doing so in #2702. will follow up with more detail so im not spamming a closed pr, just wanted to mention it here in case anyone might object to me reverting this in the near future

will follow up with more detail so im not spamming a closed pr, just wanted to mention it here in case anyone might object to me reverting this in the near future

I'm inclined to oppose reverting this as is; I think I'd rather see this reworked for what we want than move it back as-is to svm.

Right now it does two distinct things, which are the return values: ( Vec<(&'a Pubkey, &'a AccountSharedData)>, Option<Vec<&'a SanitizedTransaction>>, ).

1. It collects a list of Pubkey and the associated AccountSharedData for that Pubkey.
   • I can definitely see how this could be useful for the simd83 stuff in svm: allow conflicting transactions in entries #2702
2. It (optionally) collects transaction references to eventually pass to the geyser interface
   • This definitely has no place in SVM imo

They seem tied together right now because the filtering is identical. But I think the filtering is quite cheap (&&-ing a few bools?) - we could just redo it if needed for geyser when constructing interfaces to pass there?

Trying to think of a good way to make these separate but without duplicating the logic...